Repository navigation
test(otel): cover replay and chained execution - #796
zhongkechen wants to merge 18 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Enable cases 25 and 26 in both cloud views and the local test suite. Keep their regressions failing until the prerequisite fixes merge. Preserve the callback completion export-time assertion and both later replay barriers. Use raw span observations instead of configurable proof-file reporting, and document the intended empty-history contract. Validation: 89 focused tests pass and four expected regression cases fail. Formatting, lint, and the 63 template contract tests pass.
This comment has been minimized.
This comment has been minimized.
| assert len(targets) == 1, ( | ||
| "target completion must reach the exporter before the observed step" | ||
| ) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
yaythomas
left a comment
There was a problem hiding this comment.
per description, ready to merge once Python SDK PR #865: correct terminal telemetry status and timing and Python SDK PR #868: record callback terminal operation updates land. The inline comments are suggestions.
| assert len(targets) == 1, ( | ||
| "target completion must reach the exporter before the observed step" | ||
| ) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
| assert calls == [("target", 1)], ( | ||
| "target completion must reach the exporter before the observed step" | ||
| ) |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_ysa6hd7vcryj2bv5pbshd64jxt
[P1] Land the callback-update prerequisite before merging
The checked-out base's Execution.complete_callback_success() still does not populate updated_operation_ids. Consequently, callback completion is treated as terminal replay and its hook is omitted before the observed step, leaving the recorded export count at zero. Both parameterizations therefore fail in ordinary CI through the root testpaths and hatch run test:cov. Moving the assertion outside the step improves diagnostics but does not resolve this blocker. Land #868 and rebase before merging, preserving these regression assertions.
To suppress repeats, reply /ai verdict deferred, /ai verdict false-positive, or /ai verdict out-of-scope on the first nonblank line. Add rationale on later lines. A new /ai verdict accepted allows future repeats again.
Codex AI reviewOne merge blocker remains at Reviewed commit |
Add OpenTelemetry conformance handlers for user-function and callback contexts (22–23), invocation retries (24), caught callback-failure replay (25), external callback completion across later replays (26), and a chained child that suspends and resumes (27). Both views exercise the public Core 3 / OTel 2 APIs.
Merge prerequisite: #868 remains required for the ordinary local Case 26 tests. Keep these failures as merge gates; do not skip, xfail, or suppress them. The Case 25 fixes (#865 and #870) are already included from
main. Cases 25–26 remain active in both cloud suites and the local suite. Case 21 retains its local replay tests but is explicitlyNOT_IMPLEMENTEDin both cloud views.Validated review comments
0eaf7341c0c7ff937e6a4bf6bda1682ff07e7502, validate a full SHA, and remove unusedphase/delay_secondscontrols and pass-through.Error: {Payload: {}, Truncated: false}. README and test comments cite the run; metadata-only history remains unverified against AWS Lambda.Design
[("target", 1)]before checking history gates. Later-barrier and final raw-count assertions remain. Case 22 keeps synchronized sibling probes and checks both callback and invocation-host context restoration after execution.InvocationCompletedbeforeWaitSucceeded, while the timer can finish first. The recorded cloud run observed the reverse order on a successful execution. Increasing the wait only reduces the race. Re-enable cloud coverage after the shared requirement accepts both orders while preserving replay assertions.Workflow and cloud evidence
The reusable workflow is pinned and maintained by Dependabot. The test catalog defaults to the conformance repository's
main, withconformance_test_refretained for manual overrides. Both PR and manual runs execute invocation and execution suites. All 13 remaining caller inputs and 7 secrets match the pinned schema; its default case count is 27. Legacy long-running templates and local contract tests remain, but there are no launch/check controls in this caller.The previous four-job cloud run reported 25 passed, Case 25 failed on ERROR span status, and Case 21 NOT_IMPLEMENTED in every job. Case 26 already passed in the cloud. The saved detailed Case 25 histories verify the empty-error shape against AWS Lambda. Metadata-only
Error: {Truncated: true}is a local-runner expectation and remains unverified against the service. The updated revision's cloud qualification is pending; a green run still would not claim Case 21 coverage.The additional Case 17 callback-error regression remains outside this PR. Its source is preserved in 6484e577; correct its detailed/metadata-only history expectations before restoring it. The existing Case 17 handler remains active. The Case 11 fix is already merged in #864.
Validation
On Python 3.13.5 and Hatch 1.17.0:
10b1b31: 91 passed, 2 failed. Both failures are Case 26, reported as shifted history IDs after an assertion triggered an SDK step retry.target completion must reach the exporter before the observed step, with[("target", 0)]instead of[("target", 1)].hatch run test:all packages/aws-durable-execution-sdk-python-conformance-tests-otel/tests .github/scripts/tests/test_opentelemetry_conformance_workflow.py -qq --tb=short.Existing test expectation changes
test_templates_map_every_otel_requirement,test_templates_declare_only_the_expected_functions,test_function_names_identify_the_requirement,test_makefile_builds_every_function,test_every_registered_handler_exists,test_chained_invoke_functions_point_at_their_target,test_handler_modules_are_exactly_the_expected_set,test_active_callback_and_retry_handlers_keep_their_mapping,test_deferred_requirements_are_explicit) include active cloud cases 22–27, Case 27 targets, and retained local-only Case 21. Only Case 21 is pending.test_callback_probes_keep_their_actual_sdk_parentadds unexpected helper-failure/success cases while keeping successful-scenario parent/count assertions.test_external_callback_completion_precedes_later_replayschecks(value, export_count)outside the step; the observation point and required count remain unchanged.test_user_function_probes_keep_sdk_context_across_resumechecks recorded callback/host context identities outside SDK retry handling and requires the expected observation counts.test_opentelemetry_conformance_caller_uses_current_workflow_contractexpects a full commit SHA; a new manual-input test prevents reintroducing inactive controls.Case 27 companion PRs: JS #1068 and Java #824.